Skip to content

Conversation

SVLaursen
Copy link

Adds the Accept header of 'application/json' to the auth metadata request for the MCP client.
Reason for this being that in some cases, particularly in the SAP ecosystem, auth services have a default of going to a login page/IDP selection screen if the Accept is '/'.

Motivation and Context

Recently I've been working on getting MCP into the ERP world, particularly the SAP sphere. All apps used in this area are typically deployed to the Business Technology Platform (BTP) where auth systems like XSUAA are used.

Unfortunately with the current implementation of the auth metadata fetcher for the client side, the default logic of the XSUAA service comes into play, as it defaults to a login page when a query against the metadata endpoint is made, unless the requester explicitly asks for the JSON content.

Therefore by asking specifically for the metadata as JSON this problem can be avoided, and seeing as the subsequent logic in the client library also depends on the data received being of JSON format, it would make sense.

How Has This Been Tested?

I tested this using our internal implementation and the client returned the metadata as expected.

Breaking Changes

Not to my knowledge.

Types of changes

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to change)
  • Documentation update

Checklist

  • I have read the MCP Documentation
  • My code follows the repository's style guidelines
  • New and existing tests pass locally
  • I have added appropriate error handling
  • I have added or updated documentation as needed

Additional context

@SVLaursen
Copy link
Author

Rebasing to the latest changes. I hope you will consider this small fix as it will do wonders for integration in the SAP ecosystem.

Copy link
Member

@pcarleton pcarleton left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this change looks good overall!

if you could rebase it, that'd be great. we also just recently did a prettier pass, so hopefully the test change will be less of a formatting monster

expect(options.headers).toEqual({
"MCP-Protocol-Version": "2025-01-01"
"MCP-Protocol-Version": "2025-01-01",
Accept: "application/json",
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

note: this is the substantive part of the test change (rest of the changes in this file are formatting, we separately need to fix formatting all over so we can stop having them creep into changes)

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@pcarleton Should be rebased now with a little less over-formatting and confusing diffs 😄

@SVLaursen SVLaursen requested review from a team as code owners October 7, 2025 04:23
@SVLaursen
Copy link
Author

PR has been rebased and the changes should now be less massive with the formatting fixed

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants